Skip to content

Fix hero overlap caused by fixed header by adding layout spacing in Landing page#403

Open
Ihiechi wants to merge 2 commits intoneed4deed-org:developfrom
Ihiechi:develop
Open

Fix hero overlap caused by fixed header by adding layout spacing in Landing page#403
Ihiechi wants to merge 2 commits intoneed4deed-org:developfrom
Ihiechi:develop

Conversation

@Ihiechi
Copy link
Copy Markdown

@Ihiechi Ihiechi commented Apr 25, 2026

Description

Fixes an issue where the hero section was rendered under the fixed header.

The header uses position: fixed which removes it from normal document flow, causing the hero content to overlap.

This fix adds layout spacing in the Landing page to offset the header height and ensure proper rendering of all sections.

Related Issues

Thanks for pointing that out — you're right, this fix is not related to issue #385. I've removed the reference from the PR description.

Changes

  • Added top spacing in Landing layout to prevent hero overlap with fixed header

Screenshots / Demos

N/A (layout fix)

Checklist

  • [ x] WITHIN THE SCOPE OF AN ISSUE; No unnecessary files included
  • Tests added/updated
  • Documentation updated
  • [ x] CI passes

@ivannissimrch
Copy link
Copy Markdown
Collaborator

I noticed it's linked to close issue #385 (briefed checkmark on volunteer type tags) but the change looks like a Landing page layout fix just wanted to flag it in case the wrong issue number got linked by mistake.

Copy link
Copy Markdown
Collaborator

@nadavosa nadavosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #403 Review: Fix landing page overlap with fixed header

The bug is real and the approach is understandable, but the implementation is fragile:

1. Hardcoded 80px breaks on mobile/tablet (main issue)
The project already defines a responsive CSS variable for the header height:

  • Mobile: --layout-static-page-header-height: 64px
  • Tablet: --layout-static-page-header-height: 76px
  • Desktop: --layout-static-page-header-height: 80px

style={{ paddingTop: "80px" }} only matches the desktop value — on mobile it over-compensates by 16px, on tablet by 4px. The fix is:

style={{ paddingTop: "var(--layout-static-page-header-height)" }}

This stays in sync with the header height automatically across all breakpoints.

2. Inline style bypasses the design system
The project uses styled-components throughout. Applying layout spacing via an inline style prop is inconsistent — consider moving this padding into the AppContainer styled component or a wrapper, so it's part of the design token system.

3. PR checklist
"Tests added/updated" and "Documentation updated" are checked, but the diff is a single-line style change with no test or documentation changes.

@Ihiechi Ihiechi closed this Apr 29, 2026
@Ihiechi Ihiechi reopened this Apr 29, 2026
@Ihiechi
Copy link
Copy Markdown
Author

Ihiechi commented Apr 29, 2026

Thanks for the feedback! I've updated the implementation to use the responsive CSS variable so it adapts across breakpoints, and kept the change minimal to avoid affecting layout structure. Also corrected the checklist items.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants